-
Notifications
You must be signed in to change notification settings - Fork 14
✨ [maykinmedia/commonground-api-common#142] Use exception handler registry from commonground-api-common #723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #723 +/- ##
==========================================
- Coverage 84.98% 84.92% -0.06%
==========================================
Files 141 141
Lines 2843 2832 -11
Branches 224 222 -2
==========================================
- Hits 2416 2405 -11
Misses 375 375
Partials 52 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
f19beff to
9f8bbc0
Compare
| response = self.client.get(self.url, {"type": objecttype_url}) | ||
|
|
||
| self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, since the response is consistent with other projects, with invalid_params, etc., you can use this method to get the errors from vng_api_common.tests import get_validation_errors.
You can apply this for all tests as well.
src/objects/tests/v2/test_filters.py
Outdated
| invalid_params = {p["name"]: p["reason"] for p in data["invalid_params"]} | ||
|
|
||
| self.assertEqual( | ||
| response.json()["type"], | ||
| [ | ||
| f"Select a valid object type. {objecttype_url} is not one of the available choices." | ||
| ], | ||
| invalid_params["type"], | ||
| f"Select a valid object type. {objecttype_url} is not one of the available choices.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| invalid_params = {p["name"]: p["reason"] for p in data["invalid_params"]} | |
| self.assertEqual( | |
| response.json()["type"], | |
| [ | |
| f"Select a valid object type. {objecttype_url} is not one of the available choices." | |
| ], | |
| invalid_params["type"], | |
| f"Select a valid object type. {objecttype_url} is not one of the available choices.", | |
| self.assertEqual( | |
| get_validation_errors(response, "type"), | |
| { | |
| "name": "type", | |
| "code": "invalid_choice", | |
| "reason": f"Select a valid object type. {objecttype_url} is not one of the available choices.", | |
| }, | |
| ) |
danielmursa-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good 👍
5ee4377 to
0dd574b
Compare
6de9948 to
d42b42e
Compare
stevenbal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remarks about tests, looks good otherwise
|
| Branch | feature/142-custom-exception-handlers |
| Testbed | ubuntu-24.04 |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| performance_test/tests/test_objects_list.py::test_objects_api_list_filter_by_object_type | 📈 view plot 🚷 view threshold | 110.73 ms(-9.64%)Baseline: 122.54 ms | 128.67 ms (86.06%) |
| performance_test/tests/test_objects_list.py::test_objects_api_list_filter_one_result | 📈 view plot 🚷 view threshold | 22.02 ms(+2.41%)Baseline: 21.50 ms | 22.58 ms (97.53%) |
| performance_test/tests/test_objects_list.py::test_objects_api_list_large_page_size_page_1 | 📈 view plot 🚷 view threshold | 269.15 ms(-9.19%)Baseline: 296.38 ms | 311.20 ms (86.49%) |
| performance_test/tests/test_objects_list.py::test_objects_api_list_large_page_size_page_5 | 📈 view plot 🚷 view threshold | 270.35 ms(-8.64%)Baseline: 295.92 ms | 310.71 ms (87.01%) |
| performance_test/tests/test_objects_list.py::test_objects_api_list_small_page_size_page_20 | 📈 view plot 🚷 view threshold | 125.10 ms(-2.80%)Baseline: 128.71 ms | 135.14 ms (92.57%) |
src/objects/tests/v2/test_filters.py
Outdated
|
|
||
| data = response.json() | ||
|
|
||
| invalid_params = {p["name"]: p for p in data["invalid_params"]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| invalid_params = {p["name"]: p for p in data["invalid_params"]} | |
| error = get_validation_errors(response, "type") |
src/objects/tests/v2/test_filters.py
Outdated
| self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| data = response.json() | ||
| invalid_params = {p["name"]: p["reason"] for p in data["invalid_params"]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OlhaZahoruiko I think you can use this method everywhere to get the error dict
@stevenbal should we also add the key for validations that are not provided? like here
objects-api/src/objects/api/validators.py
Line 93 in ff5f242
| raise serializers.ValidationError(message, code=code) |
| invalid_params = {p["name"]: p["reason"] for p in data["invalid_params"]} | |
| error = get_validation_errors(response, "") |
f26f6b6 to
178c973
Compare
| ["Fields in the configured authorization are absent in the data: 'some'"], | ||
| data = response.json() | ||
|
|
||
| reasons = [p["reason"] for p in data["invalid_params"]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here:
| reasons = [p["reason"] for p in data["invalid_params"]] | |
| error = get_validation_errors(response, "") |
|
|
||
| data = response.json() | ||
|
|
||
| self.assertEqual( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.assertEqual( | |
| error = get_validation_errors(response, "") | |
| self.assertEqual(error["reason"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you forget this one
src/objects/tests/v2/test_filters.py
Outdated
| response.json(), ["Operator `lt` supports only dates and/or numeric values"] | ||
|
|
||
| data = response.json() | ||
| reasons = [p["reason"] for p in data["invalid_params"]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| reasons = [p["reason"] for p in data["invalid_params"]] | |
| error = get_validation_errors(response, "") |
|
|
||
| data = response.json() | ||
|
|
||
| self.assertEqual( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.assertEqual( | |
| error = get_validation_errors(response, "") | |
| self.assertEqual( |
| [ | ||
| "'date' and 'registrationDate' parameters can't be used in the same request" | ||
| ], | ||
| data["invalid_params"][0]["reason"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| data["invalid_params"][0]["reason"], | |
| error = get_validation_errors(response, "") | |
| data["invalid_params"][0]["reason"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And also here
| self.assertEqual( | ||
| data["invalid_params"][0]["name"], | ||
| "date", | ||
| ) | ||
| self.assertEqual( | ||
| data["invalid_params"][0]["reason"], | ||
| "Enter a valid date.", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.assertEqual( | |
| data["invalid_params"][0]["name"], | |
| "date", | |
| ) | |
| self.assertEqual( | |
| data["invalid_params"][0]["reason"], | |
| "Enter a valid date.", | |
| ) | |
| error = get_validation_errors(response, "date") |
| self.assertEqual( | ||
| data["invalid_params"][0]["name"], | ||
| "registrationDate", | ||
| ) | ||
| self.assertEqual( | ||
| data["invalid_params"][0]["reason"], | ||
| "Enter a valid date.", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.assertEqual( | |
| data["invalid_params"][0]["name"], | |
| "registrationDate", | |
| ) | |
| self.assertEqual( | |
| data["invalid_params"][0]["reason"], | |
| "Enter a valid date.", | |
| ) | |
| error = get_validation_errors(response, "registrationDate") |
| [ | ||
| "'fields' query parameter has invalid or unauthorized values: 'someField'" | ||
| ], | ||
| data["invalid_params"][0]["reason"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| data["invalid_params"][0]["reason"], | |
| error = get_validation_errors(response, "") | |
| data["invalid_params"][0]["reason"], |
178c973 to
ff73c5f
Compare
src/objects/tests/v2/test_filters.py
Outdated
|
|
||
| error = get_validation_errors(response, "") | ||
|
|
||
| self.assertIn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self.assertIn( | |
| self.assertEqual( |
…istry from commonground-api-common
ff73c5f to
270fa43
Compare
Fixes maykinmedia/commonground-api-common#142
Changes
[Describe the changes here]